feat(server): engine.commands defense-in-depth consumer (PR 5) - #3
Conversation
PR 3 made agent.template.sync.jobs the canonical path for "agents need to re-pull template:meta:*". This change adds a defense-in-depth consumer on the legacy engine.commands queue so any producer that still publishes "internal:template upload" / "internal:template delete" there (stale code paths, third-party integrations) gets routed back into the same notify-agents helper instead of being silently dropped. - New EngineCommandQueueProcessor mirrors TemplateSyncQueueProcessor's shape (Listen goroutine, Stop hook, context-scoped processing). - isTemplateNotifyCommand keeps command classification in a tiny pure helper so the routing logic is unit-testable without RabbitMQ. - Unknown commands (engine health pings, scan dispatch, etc.) are ack-and-ignored so the consumer doesn't fight other producers. - Wired into server.NewServer alongside the existing template-sync consumer; failures only log and never abort startup.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bee1709f4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Other producers (engine health pings, scan dispatch, | ||
| // etc.) legitimately share this queue. Ack-and-ignore. | ||
| ecp.logger.Debug("Unhandled engine.commands command", | ||
| zap.String("command", cmd.Command)) |
There was a problem hiding this comment.
Do not discard non-template engine.commands messages
This new consumer subscribes to engine.commands and explicitly treats unknown commands as "Ack-and-ignore"; on a shared RabbitMQ queue, that means non-template messages can be consumed here and silently dropped instead of reaching the component that actually handles them. The risk is highest in deployments where other command types (e.g. scan/health events noted in the comment) are published to the same queue, because those events may disappear intermittently once this service is running.
Useful? React with 👍 / 👎.
Summary
Sprint PR 5 of the scanner-templates-fix playbook (in the Sirius repo).
PR 3 made `agent.template.sync.jobs` / `notify_agents` the canonical path for "agents need to re-pull `template:meta:*`". This PR adds a defense-in-depth consumer on the legacy `engine.commands` queue so any producer that still publishes `internal:template upload` / `internal:template delete` there (stale code paths, third-party integrations) gets routed back into the same notify-agents helper instead of being silently dropped.
Change set
Verification (post-deploy)
```bash
docker exec sirius-rabbitmq rabbitmqadmin publish \
exchange=amq.default routing_key=engine.commands \
payload='{"command":"internal:template upload","template_id":"smoke-test","timestamp":"2026-04-22T00:00:00Z"}'
```
Agent log shows `Routing engine.commands template event to notify-agents`.
Risk
Low. Strictly additive; failure modes are scoped to the new consumer.
Out of scope
Replacing `engine.commands` with a typed exchange/event bus (deferred Phase C).